Skip to content

perf(scan): reuse loaded global index planning context - #267

Merged
lxy-9602 merged 10 commits into
apache:mainfrom
wangyong9999:perf/reuse-index-snapshot-caches
Sep 1, 2026
Merged

perf(scan): reuse loaded global index planning context#267
lxy-9602 merged 10 commits into
apache:mainfrom
wangyong9999:perf/reuse-index-snapshot-caches

Conversation

@wangyong9999

@wangyong9999 wangyong9999 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: N/A

DataEvolutionBatchScan used GlobalIndexScan::Create() while planning an index predicate. That public entry point reloaded the latest table schema and snapshot and constructed its own planning context, even though TableScan had already resolved the same state.

The DataEvolution scan now builds its internal GlobalIndexScanImpl from the schema, snapshot manager, CoreOptions, filesystem, executor, memory pool, and caller-provided cache already owned by the current TableScan. The index and data planning stages therefore use the same scan context, and repeated plans can hit the cache supplied through ScanContextBuilder::WithCache().

This PR does not retain GlobalIndexReader objects or BTree pages across queries and adds no process-global registry. Cross-query reader ownership, eviction, memory accounting, and IsThreadSafe() handling remain query-engine responsibilities through the existing public GlobalIndexScan, CreateReader, and SetGlobalIndexResult APIs.

Tests

  • paimon-global-index-test: 132/132 passed.
  • FileFormat/GlobalIndexTest.TestDataEvolutionBatchScan/*: 6/6 passed.
  • The DataEvolution integration case verifies that a caller-provided manifest cache has no new supplier calls on the second scan.
  • git diff --check passes.

The local environment does not have pre-commit installed; GitHub Actions cover the repository lint hooks.

API and Format

No public API, storage-format, or protocol change. The modified constructor is internal to the core implementation.

Documentation

No user-facing option or behavior change.

Generative AI tooling

Generated-by: Codex (GPT-5)

@wangyong9999

wangyong9999 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

The failure came from process-wide BTree pages warmed by an earlier fixture. The cache key now includes the backend namespace, so cross-fixture pages cannot be reused. I restored the exact file-open assertion; all 12 primary-key sorted-index integration cases pass for Parquet and ORC. The latest fork workflows are waiting for maintainer approval.

Comment thread src/paimon/common/global_index/btree/btree_global_indexer.cpp Outdated
Comment thread src/paimon/common/global_index/btree/btree_global_indexer.cpp Outdated
Comment thread src/paimon/core/operation/file_store_scan.cpp Outdated
Comment thread src/paimon/core/operation/file_store_scan.cpp Outdated
@lxy-9602

lxy-9602 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Thanks for working on the repeated Global Index lookup performance.

Some downstream query engines, such as StarRocks, have a similar requirement. Their approach is to create a GlobalIndexReader through the public API in global_index_scan.h and retain/cache the reader for multiple queries. This already preserves the BTree reader's internal block cache without introducing process-wide cache sharing inside paimon-cpp.

The process-wide design in this PR introduces additional concepts and hidden global state, including the shared CacheManager registry, cache configuration LRU, cache_namespace, backend lifetime retention, and cross-reader allocator management. It also makes memory accounting and cache isolation harder for downstream engines that already have their own resource management.

Could we instead encourage the query engine to split the query into two explicit stages?

  1. Global Index stage: create or reuse a GlobalIndexReader through GlobalIndexScan, evaluate the index predicate, and produce a GlobalIndexResult. This stage can be distributed by shard.
  2. Data Scan stage: pass the returned GlobalIndexResult through ScanContextBuilder::SetGlobalIndexResult(), then use TableScan to generate the data scan plan and execute it.

The engine can cache readers using a key containing the table identity, snapshot/index generation, field, index type, shard/partition, row range, and relevant options. It should also ensure that the index lookup and subsequent data scan use the same snapshot, and respect GlobalIndexReader::IsThreadSafe() when sharing readers.

This keeps cache ownership, eviction, memory accounting, and isolation under the query engine's control while reusing the existing public API. Could we evaluate this approach before adding process-wide cache sharing to paimon-cpp?

@wangyong9999 wangyong9999 changed the title perf(scan): reuse index planning resources across queries perf(scan): reuse loaded global index planning context Sep 1, 2026
@wangyong9999

Copy link
Copy Markdown
Contributor Author

Agreed. I removed the process-wide reader and BTree cache registry, namespace, and allocator-lifetime changes. The query engine now owns cross-query GlobalIndexReader reuse through the existing public API, pins the data scan to the same snapshot, and honors IsThreadSafe() around shared-reader evaluation.

This PR is now limited to reusing the schema, snapshot context, executor, memory pool, and caller-provided manifest cache already available inside one TableScan. The resulting diff is 4 files; paimon-global-index-test passes 132/132.

@lxy-9602 lxy-9602 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@lxy-9602
lxy-9602 merged commit c3a6d83 into apache:main Sep 1, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants